Skip to content

Upgrade to Typescript 5.x and Svelte 4.x #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

patricknelson
Copy link

@patricknelson patricknelson commented Aug 16, 2023

Possible fix for #93. Quick hack that upgrades TypeScript v4 -> v5 and Svelte v3 -> v4.

I actually did this in an attempt to debug unrelated issues which ended up being a red herring. However, since I already hashed through all of the linting and TypeScript errors, I figured maybe you could benefit from this! Again, this is an ugly hack (particularly just the errors I throw, which I'll call out below). However, this may save you some time later if/when you have time to address this.

Thankfully, Svelte 4 is almost entirely backward compatible with v3, the main differences here where how they defined their types and how they need to be imported (thus the updates to the import statements and .eslintrc.js)

@patricknelson patricknelson force-pushed the upgrade-typescript5-svelte4 branch from 1e18d98 to 5d5741e Compare August 16, 2023 05:19
@@ -46,8 +46,10 @@ export default class Processor {
this.magicContent = new MagicString(content);
this.styleParser = parser.bind(this);

if (!this.ast.css) throw new Error('ast.css is not defined.');
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hack: Needed to do this to pass linting since the type of Ast.css changed to be nullable.

@@ -59,6 +61,8 @@ export default class Processor {
* @returns The generated module classname
*/
public createModuleClassname = (name: string): string => {
if (!this.ast.css) throw new Error('ast.css is not defined.');
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hack: Needed to do this to pass linting since the type of Ast.css changed to be nullable.

@@ -123,6 +127,7 @@ export default class Processor {
const child = item.children[0];
const name = child.name ?? child.value.replace(/'|"/g, '');
const varName = child.type === 'String' ? name.replace(/\./, '-') : name;
if (!this.ast.css) throw new Error('ast.css is not defined.');
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hack: Needed to do this to pass linting since the type of Ast.css changed to be nullable.

@@ -38,14 +35,15 @@ let pluginOptions: PluginOptions;
* @returns the preprocessor markup
*/
const markup: MarkupPreprocessor = async ({ content, filename }) => {
const isIncluded = isFileIncluded(pluginOptions.includePaths, filename);
const useFilename: string = filename || '';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hack? This filename parameter also changed to potentially undefined.

@mindmind
Copy link
Contributor

waiting for merge

@micantoine
Copy link
Owner

@patricknelson Thank you for you work.

When looking at the PR #100 , I ended up encountering the issues you faced on this current PR. I tackled the changes over there based on what you did here.

Here are some noticeable differences:

  • I removed typescript as peer dependencies, this is actually not needed since the package works without. It is only necessary when developing the preprocessor. (Thus fixing the typescript version)
  • Since I still want to support svelte3, both v3 and v4 are in the peer dependencies
  • The [email protected] only support ESM which is not the case with this preprocessor. I had to downgrade to v2 in order to still be compatible with CJS

v2.2.5 of the svelte-preprocess-cssmodules package has been released.

ps: My apologies for not being able to maintain the plugin for about a year

@micantoine micantoine closed this Sep 19, 2024
@patricknelson
Copy link
Author

Awesome! And no worries. I see you’ve been heads down working through the issues and cranking out code for at least the past 12 hours or so. Our team uses this plugin regularly, so we love it and we don’t want you burning out! Thanks for the dedication. 😊

@patricknelson patricknelson deleted the upgrade-typescript5-svelte4 branch September 19, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants